Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

entry: convert hard link absolute paths to relative ones #248

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gdesmott
Copy link

When unpacking a docker image, some hard links may be pointing to, say,
/bin/gunzip. Those were failing the validate_inside_dst() check,
preventing to unpack the image.

Fix this by re-using the same logic as we are already using for actual
files, removing the leading '/' from the link target path.

We do this only if the absolute link is not an ancestor of the target
dir as we want to support this use case according to the existing
absolute_hardlink() test.

src/entry.rs Outdated Show resolved Hide resolved
@alexcrichton
Copy link
Owner

Looks good to me! I think tests may be failing now though :(

@gdesmott
Copy link
Author

All tests are passing here (Linux) and I don't have access to a macos system to debug this I'm afraid. :\

@gdesmott
Copy link
Author

I got access to a macos box from a colleague. Here is the problem:

[src/entry.rs:485] &dest_canon = "/private/var/folders/v3/fb_smfzj3bn41k7jj7vfqqh80000gr/T/tarampqNh"
[src/entry.rs:488] src.components() = Components(
    [
        RootDir,
        Normal(
            "var",
        ),
        Normal(
            "folders",
        ),
        Normal(
            "v3",
        ),
        Normal(
            "fb_smfzj3bn41k7jj7vfqqh80000gr",
        ),
        Normal(
            "T",
        ),
        Normal(
            "tarampqNh",
        ),
        Normal(
            "foo",
        ),
    ],
)
[src/entry.rs:492] &src = "var/folders/v3/fb_smfzj3bn41k7jj7vfqqh80000gr/T/tarampqNh/foo"

As you can see Path::components is parsing /private instead of / as root. Is that normal or a bug in the parsing? (I have 0 experience with macos)

@alexcrichton
Copy link
Owner

Sorry I'm not sure, I've never seen this behavior before.

@gdesmott
Copy link
Author

Sorry I got confused while debugging. The actual issue is test's Tempdir being created as /var/folders/... which is canonicalized to /private/var/folders/... as /var is a symlink to /private/var on macos.
The unpack dest dir was canonicalized but the not the target dir, hence the test failing.

I solved it by canonicalizing the tempdir as well.

@alexcrichton
Copy link
Owner

I think windows may now be failing?

When unpacking a docker image, some hard links may be pointing to, say,
/bin/gunzip. Those were failing the validate_inside_dst() check,
preventing to unpack the image.

Fix this by re-using the same logic as we are already using for actual
files, removing the leading '/' from the link target path.

We do this only if the absolute link is not an ancestor of the target
dir as we want to support this use case according to the existing
absolute_hardlink() test.
@gdesmott
Copy link
Author

god, working with paths isn't fun on Windows. canonicalize() returning the extended syntax version of the path makes everything much harder. Anyway, I think I managed to make it work .

@gdesmott
Copy link
Author

That's really strange, the test is passing on my Windows box so I'm afraid there is not much more I can do. :\

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants